Skip to content

Conversation

@teogeb
Copy link
Contributor

@teogeb teogeb commented Nov 22, 2024

Added maxSize and maxAge options to the Mapping class. These are needed in follow-up PR (#2903).

Added two builder functions: createCacheMap() and createLazyMap(). This way the user of the class needs to decide if the Mapping if is a cache or just a lazy loading map. For caches it is required to configure maxSize.

Type safety

The constructor of Mapping is annotated as @internal so that in encourages the usage of the builder functions. If we make it private, the builder functions needs to have access to it somehow. Could e.g. be static methods. Or we could export Mapping as a type.

Testing

In unit tests we test the basic functionality for the lazy map, and use cache map only test the size limitations. This is maybe ok as in the Mapping the differences are abstracted out to the delegate (which is either a LRU cache or plain Map).

Future improvements

  • Use Mapping instead of CacheMap in StreamRegistry/StreamStorageRegistry and remove the CacheMap class

@teogeb teogeb marked this pull request as ready for review November 25, 2024 14:34
@teogeb teogeb merged commit 742b7ff into main Nov 26, 2024
23 checks passed
@teogeb teogeb deleted the sdk-mapping-cache-constraints branch November 26, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants